Skip to content

feat(button): rework component on top of @base-ui/react - #383

Open
H4ad wants to merge 1 commit into
feat/add-spinner-componentfrom
feat/rework-button-component
Open

feat(button): rework component on top of @base-ui/react#383
H4ad wants to merge 1 commit into
feat/add-spinner-componentfrom
feat/rework-button-component

Conversation

@H4ad

@H4ad H4ad commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What

This reworks the button to follow the guidelines of https://github.com/SigNoz/engineering-pod/issues/5983

Closes https://github.com/SigNoz/engineering-pod/issues/6010

Component checklist

Philosophy (guidelines)

  • Variants represent meaningful roles (purpose), not one-off visual styling
  • All applicable states handled: default, hover, active, selected, focus, disabled, loading, invalid, read-only
  • Accessibility encoded in component: keyboard, focus, labels, ARIA (not left to consumer)
  • Complex behaviours split into composable patterns, not crammed into conflicting props
  • Escape hatches minimized; CSS overrides are last resort (if exception recurs, evolve the component)

Structure (guidelines)

  • Directory is kebab-case under packages/ui/src/, matches the export subpath
  • index.ts only re-exports (+ generated token region); export type * plus explicit named exports
  • Relative imports use the .js extension; types imported with import type
  • Subcomponents in subcomponents/, opinionated compositions in presets/
  • All four lists updated: src/index.ts, vite.config.ts, package.json exports, README.md + intro.mdx
  • New runtime deps added to externalPatterns

CSS (guidelines)

  • {name}.module.scss, no Tailwind, no CVA, no Sass variables
  • Every overridable value is var(--{component}-x, <design token>); no hardcoded values
  • No literal fallback on a design token (var(--spacing-4, 8px)); a bare literal only where no token matches
  • Colours use semantic tokens only, with no --bg-* / --text-* primitives referenced from a component
  • Values that must not be overridden use --{component}-internal-*
  • No --x: var(--x) and no duplicated variable definitions
  • Variants are data-* attributes; data-slot and data-testid on the root
  • color-mix() for derived shades, never rgba() over a token
  • :hover, :focus-visible, aria-invalid, disabled all styled
  • Transitions use the component's duration/easing tokens and are disabled under @media (prefers-reduced-motion: reduce)
  • pnpm run tokens run and the index.ts region committed

Visual QA (guidelines)

  • Compared against the Figma frame side by side: spacing, sizes, colours, every state
  • Figma frame linked from the story via parameters.design
  • Checked in light and dark
  • Sizing and density consistent with sibling components (sm matches Button / Input)
  • Typography via Typography or the type-scale tokens, not an ad-hoc font-size / font-weight
  • Icons from @signozhq/icons, sized with tokens, no inline SVG with hardcoded px
  • Every state built: default, hover, focus-visible, active, disabled, loading, invalid, selected, empty, long/truncated
  • Not a duplicate of an existing primitive or preset

Types (guidelines)

  • {Component}Props exported; only the props the component actually needs are exposed
  • Upstream-owned prop types borrowed by indexed access (OriginalProps['x']), never restated by hand
  • Every type named by a public prop is exported from index.ts
  • forwardRef + displayName; asChild, testId supported
  • Defaults in the destructuring; controlled/uncontrolled naming follows Radix

Docs (guidelines)

  • JSDoc on every public prop, with @default where applicable
  • Story file per exported component, correct title group
  • argTypes complete with category, type.summary, defaultValue.summary
  • Stories for every meaningful state, not just the happy path
  • {component}.mdx with a usage snippet and a <Controls> per exported piece

Tests (guidelines)

  • Behaviour test per interactive prop
  • forward-ref test
  • run-visual-testing label added if the change is visual

@H4ad
H4ad force-pushed the feat/rework-button-component branch from 2995958 to b518ac4 Compare September 6, 2026 23:54
@H4ad
H4ad force-pushed the feat/rework-button-component branch from b518ac4 to 1e231f0 Compare September 7, 2026 01:02
@H4ad
H4ad changed the base branch from feat/rework-tooltip-component to feat/add-spinner-component September 7, 2026 20:56
@H4ad
H4ad force-pushed the feat/rework-button-component branch from 1e231f0 to f3c69ce Compare September 7, 2026 20:56
@H4ad
H4ad marked this pull request as ready for review September 7, 2026 20:56
@H4ad
H4ad marked this pull request as draft September 7, 2026 21:01
@H4ad
H4ad force-pushed the feat/rework-button-component branch 3 times, most recently from b0a5be7 to c1f42a7 Compare September 8, 2026 14:22

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rework this later entirely, for now, we can keep this basic doc, this will change a lot after the doc proposal/layout


## How to use

`variant`, `color`, `size` and the children are all required: the button has no default

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this intentionally to help agents figure out what that button represents and how it looks like

@H4ad
H4ad force-pushed the feat/rework-button-component branch from c1f42a7 to 5f31a2d Compare September 8, 2026 15:50
@H4ad
H4ad marked this pull request as ready for review September 8, 2026 20:23
@H4ad
H4ad force-pushed the feat/rework-button-component branch from 5f31a2d to 9c27d1e Compare September 8, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant